Skip to content

COMP: Fix DCMTK build-tree export for external ITK consumers#6543

Open
hjmjohnson wants to merge 3 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:fix-dcmtk-build-export-namespace
Open

COMP: Fix DCMTK build-tree export for external ITK consumers#6543
hjmjohnson wants to merge 3 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:fix-dcmtk-build-export-namespace

Conversation

@hjmjohnson

Copy link
Copy Markdown
Member

ITK's FetchContent-built DCMTK writes a build-tree DCMTKTargets.cmake whose imported targets reference the nonexistent DCMTK::ITK::ITKZLIBModule (and TIFF/JPEG/PNG variants), so external consumers of an ITK build tree (e.g. an ANTs SuperBuild) fail at configure. Three commits: repair the export references in place, link namespaced DCMTK:: targets in the IO modules, and require DCMTK ≥ 3.7.0 for ITK_USE_SYSTEM_DCMTK.

Root cause
  • DCMTK generates its build-tree export with the legacy export(TARGETS ... NAMESPACE DCMTK::), which blindly prefixes DCMTK:: onto ITK's already-namespaced codec targets passed in via set(ZLIB_LIBRARIES ITK::ITKZLIBModule) etc. The install-tree export, written by install(EXPORT), resolves the same dependencies correctly to ITK::*Module.
  • A consumer whose find_package(ITK) scope has ITK_BINARY_DIR unset (any external project, notably SuperBuilds) runs find_package(DCMTK) from ITKDCMTK.cmake and imports the broken targets: set_target_properties: target DCMTK::ITK::ITKZLIBModule not found.
  • Independently, ITKIODCMTK/IOTransformDCMTK linked the bare in-scope dcmdata… targets, so the exported link interface carried $<LINK_ONLY:dcmdata> — unresolvable outside ITK's own build; consumers then failed at link with ld: library not found for -ldcmdata. (The install export mapped these to DCMTK::dcmdata; only the build export kept them bare.)

The fix patches the imported targets' INTERFACE_LINK_LIBRARIES in place after the existing find_package(DCMTK), preserving DCMTK_DIR seeding and all DCMTKConfig side effects (DCMTK_FOUND, DCMTK_INCLUDE_DIRS, …), and correcting the targets regardless of whether the consumer imported DCMTK before or after ITK.

Downstream testing performed

All against a full ITK build (Module_ITKIODCMTK=ON, ITK_BUILD_ALL_MODULES=ON, macOS arm64, clang 19, CMake 4.3):

  1. External build-tree consumerfind_package(ITK REQUIRED COMPONENTS ITKIODCMTK) with ITK_BINARY_DIR='': configures, builds, links, and runs (itk::DCMTKImageIO::New(), rc=0). DCMTK_FOUND=1, DCMTK_DIR and DCMTK_INCLUDE_DIRS populated. Before the fix: configure error on DCMTK::ITK::ITKZLIBModule; with only the export repair: link error -ldcmdata not found.
  2. Import-order variantfind_package(DCMTK) against the ITK build root before find_package(ITK) (SuperBuild pattern): probes confirm the broken DCMTK::ITK::ITKZLIBModule reference is present after the first import and rewritten to ITK::ITKZLIBModule after find_package(ITK); build+link+run OK.
  3. Downstream project forest — ANTs SuperBuild (the original failure report) and elastix configure and build green against the fixed ITK build tree. Other forest members' failures were verified unrelated (pre-existing zlib-mode env contamination and ITK-main API drift in c3d/SimpleITK/remote modules).
  4. Regressionctest -R DCMTK: 48/50 pass; the 2 failures are a pre-existing local KWStyle binary rpath issue (aborts before checking code). pre-commit run --all-files clean on the tip.
Commit breakdown
Commit Topic
COMP: Require DCMTK 3.7.0 or later for ITK_USE_SYSTEM_DCMTK Match the vendored DCMTK version; guarantees DCMTK::-namespaced system package targets
COMP: Link namespaced DCMTK targets in the DCMTK IO modules Exported link interface carries resolvable DCMTK::dcmdata…; pins BUILD_SINGLE_SHARED_LIBRARY=OFF and retires dead suffix handling
COMP: Repair DCMTK build-tree export references for external consumers In-place DCMTK::ITK::*ModuleITK::*Module rewrite after find_package(DCMTK)

Match the vendored DCMTK version and guarantee that the system package
config exports DCMTK::-namespaced imported targets, which the DCMTK IO
modules link against.
Bare dcmdata/dcmimgle/... names survive only inside ITK's own build
scope; the exported link interface then carries $<LINK_ONLY:dcmdata>,
which no external consumer of the ITK build tree can resolve. The
DCMTK:: wrapper targets export as DCMTK::dcmdata, which consumers
resolve through find_package(DCMTK).

Pin BUILD_SINGLE_SHARED_LIBRARY=OFF for the vendored build: a single-
library build renames the per-library targets with a suffix that the
DCMTK::<lib> link names cannot match, and the suffix variable is not
visible outside DCMTK's directory scope. This also retires the dead
DCMTK_LIBRARY_SUFFIX handling in the wrapper loop.
DCMTK's build-tree DCMTKTargets.cmake is written by the legacy
export(TARGETS ... NAMESPACE DCMTK::), which prefixes DCMTK:: onto
ITK's imported codec targets and records the nonexistent
DCMTK::ITK::ITKZLIBModule (and TIFF/JPEG/PNG). An external consumer of
an ITK build tree that resolves the DCMTK targets (e.g. an ANTs
SuperBuild, where ITK_BINARY_DIR is unset in the find_package(ITK)
scope) fails at generate with 'target DCMTK::ITK::ITKZLIBModule not
found'. The install-tree export, written by install(EXPORT), resolves
the same dependencies to their real ITK::* names and is unaffected.

Rewrite the mis-prefixed link-interface entries in place after the
existing find_package(DCMTK) import. Patching after import preserves
the DCMTK_DIR seeding and every DCMTKConfig side effect (DCMTK_FOUND,
DCMTK_INCLUDE_DIRS, ...), and corrects the targets regardless of
whether the consumer imported them before or after find_package(ITK).

@dzenanz dzenanz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good on a glance.

@github-actions github-actions Bot added type:Compiler Compiler support or related warnings type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:IO Issues affecting the IO module area:ThirdParty Issues affecting the ThirdParty module labels Jul 2, 2026
@hjmjohnson hjmjohnson marked this pull request as ready for review July 3, 2026 00:16
@greptile-apps

greptile-apps Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes DCMTK build-tree exports for external ITK consumers. The main changes are:

  • Link DCMTK IO modules through DCMTK:: namespaced targets.
  • Require system DCMTK 3.7.0 or newer for ITK_USE_SYSTEM_DCMTK.
  • Rewrite malformed vendored DCMTK build-tree codec dependencies from DCMTK::ITK::*Module to ITK::*Module.

Confidence Score: 4/5

This PR is close to safe, with one contained build-configuration issue to fix.

The export repair and namespaced target changes match the intended external-consumer flow, while an existing BUILD_SINGLE_SHARED_LIBRARY=ON cache value can bypass the new pin and leave the changed DCMTK::dcmdata links unresolved.

Modules/ThirdParty/DCMTK/CMakeLists.txt

T-Rex T-Rex Logs

What T-Rex did

  • Ran a real ITK vendored DCMTK configure with BUILD_SINGLE_SHARED_LIBRARY ON; CMakeCache showed the value rewritten to INTERNAL=OFF.
  • Executed a minimal CMake harness that exercises the cache and target-creation logic, validating per-library target creation when BUILD_SINGLE_SHARED_LIBRARY is OFF in the cache semantics.
  • Ran a two-step harness that started with a cache seeded as BUILD_SINGLE_SHARED_LIBRARY=ON and reconfigured to observe changes to the cache entry, which changed to INTERNAL=OFF.
  • Reviewed DCMTK build-tree consumer artifacts to understand when and where the consumer failed or halted, noting that the post-run head did not complete the expected rewrite.
  • Verified that the head run did not rewrite the interface to ITK::ITKZLIBModule and observed that the consumer stops at find_package, indicating a contract mismatch before a successful build.
  • Observed that the build's IO/DCMTK module collection shifted from bare libraries to DCMTK::-prefixed targets and IOTransformDCMTK mapping, but end-to-end validation could not complete due to a not-yet-finished libITKIODCMTK-6.0.so.1 dependency.
  • Ran DCMTK system-version checks; found that fake DCMTK 3.7.0 satisfies the version constraint while 3.6.8 triggers an incompatible version error.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
Modules/IO/DCMTK/src/CMakeLists.txt Updates ITKIODCMTK to link namespaced DCMTK targets so exported build-tree consumers resolve DCMTK libraries.
Modules/IO/IOTransformDCMTK/src/CMakeLists.txt Updates IOTransformDCMTK to link the namespaced DCMTK dcmdata target.
Modules/ThirdParty/DCMTK/CMakeLists.txt Adds vendored DCMTK namespaced wrappers and build-tree export repair, with one cache override issue in the single-library mode pin.
Modules/ThirdParty/DCMTK/itk-module-init.cmake Raises system DCMTK discovery to require version 3.7.0 for namespaced target availability.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
  participant Consumer as External build-tree consumer
  participant ITK as ITKConfig / ITKDCMTK export code
  participant DCMTK as DCMTKConfig.cmake
  participant Targets as Imported targets

  Consumer->>ITK: find_package(ITK COMPONENTS ITKIODCMTK)
  ITK->>DCMTK: set DCMTK_DIR to ITK build root
  ITK->>DCMTK: find_package(DCMTK REQUIRED NO_MODULE)
  DCMTK->>Targets: import DCMTK::dcmdata and related targets
  ITK->>Targets: "rewrite DCMTK::ITK::*Module links to ITK::*Module"
  Consumer->>Targets: "link ITKIODCMTK / IOTransformDCMTK via DCMTK::* targets"
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
  participant Consumer as External build-tree consumer
  participant ITK as ITKConfig / ITKDCMTK export code
  participant DCMTK as DCMTKConfig.cmake
  participant Targets as Imported targets

  Consumer->>ITK: find_package(ITK COMPONENTS ITKIODCMTK)
  ITK->>DCMTK: set DCMTK_DIR to ITK build root
  ITK->>DCMTK: find_package(DCMTK REQUIRED NO_MODULE)
  DCMTK->>Targets: import DCMTK::dcmdata and related targets
  ITK->>Targets: "rewrite DCMTK::ITK::*Module links to ITK::*Module"
  Consumer->>Targets: "link ITKIODCMTK / IOTransformDCMTK via DCMTK::* targets"
Loading

Comments Outside Diff (1)

  1. General comment

    P1 Build-tree ITK import skips DCMTK target rewrite after DCMTK was imported first

    • Bug
      • The PR claim requires an external build-tree consumer to import DCMTK from the ITK build root before find_package(ITK), then have ITK's package import rewrite already-imported DCMTK:: targets from DCMTK::ITK::*Module references to ITK::*Module. Executed head evidence shows this does not happen: after find_package(ITK REQUIRED COMPONENTS ITKIODCMTK), DCMTK::dcmdata still contains DCMTK::ITK::ITKZLIBModule. This leaves the same broken imported-target namespace that the change was intended to repair.
    • Cause
      • In Modules/ThirdParty/DCMTK/CMakeLists.txt, the generated build-tree export code added at lines 172-187 is wrapped in if(NOT ITK_BINARY_DIR). In the build-tree consumer scenario, importing ITK defines ITK_BINARY_DIR, so the rewrite loop is skipped and previously imported DCMTK:: targets are not repaired.
    • Fix
      • Run the rewrite loop for external consumers even when ITK_BINARY_DIR is defined by a build-tree ITK package import. For example, do not guard the repair with if(NOT ITK_BINARY_DIR), or replace it with a condition that only avoids recursive/inside-ITK execution while still executing during external build-tree package import. Then verify that DCMTK::ITK:: entries become ITK:: after find_package(ITK).

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "COMP: Repair DCMTK build-tree export ref..." | Re-trigger Greptile

set(BUILD_APPS OFF)
# Per-library targets only: a single-library build renames targets with a
# suffix the DCMTK IO modules' DCMTK::<lib> link names cannot match.
set(BUILD_SINGLE_SHARED_LIBRARY OFF CACHE INTERNAL "")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Cache value not forced
set(BUILD_SINGLE_SHARED_LIBRARY OFF CACHE INTERNAL "") does not replace an existing cache entry, so a configure that already has BUILD_SINGLE_SHARED_LIBRARY=ON keeps DCMTK in single-library mode. The code below then looks for unsuffixed per-library targets like dcmdata, finds none, and the IO modules now link DCMTK::dcmdata, which is never created.

Suggested change
set(BUILD_SINGLE_SHARED_LIBRARY OFF CACHE INTERNAL "")
set(BUILD_SINGLE_SHARED_LIBRARY OFF CACHE INTERNAL "" FORCE)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just do set(BUILD_SINGLE_SHARED_LIBRARY OFF) it should not be a cache variable.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. A plain set(BUILD_SINGLE_SHARED_LIBRARY OFF) in the parent scope will shadow any cache entry of that name when CMake evaluates it in the FetchContent subdirectory, which is the correct and idiomatic way to force options on FetchContent dependencies. The CACHE INTERNAL form doesn't force-overwrite an existing cache entry (that requires FORCE), making it unreliable.

Suggested change
set(BUILD_SINGLE_SHARED_LIBRARY OFF CACHE INTERNAL "")
set(BUILD_SINGLE_SHARED_LIBRARY OFF)

@blowekamp

Copy link
Copy Markdown
Member

I was able to inspect the DCMTKTargets.cmake file and verified this but.

The first two commit look like they can quickly go in quickly.

The namespace export changes I would like more time to inspect closer, there may be changes needed in DCMTK to make this fix more normal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:IO Issues affecting the IO module area:ThirdParty Issues affecting the ThirdParty module type:Compiler Compiler support or related warnings type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants